[#746] establish default order for replicas listed by an `iRODSDataObject` by d-w-moore · Pull Request #815 · irods/python-irodsclient

d-w-moore · 2026-04-15T22:55:22Z

The parent data object's modify_time and replica_status fields , as well as some others, actually pertain more to individual replicas.

#747 was an old PR meant to address the issue and contains much discussion as well.

On consideration, I think a minor release is the proper place to address this, and I'm doing it by

setting a default sorter that for data_objects.get ( or anytime running the iRODSDataObject constructor) sorts replicas of the data object first by the replica-"goodness" and secondly by reverse chronology of the replica modify_time (ie most recent first.) The replica at array position [0] will then determine the values of the fields discussed above.
deciding for the time being not to deprecate anything. yet. To me it makes natural sense to allow modify_time and replica_status to be accessed from the "head" object.

So, this PR replaces the old one, #747 , due to being new work and being based on top of source code conveniently ruff-formatted.

korydraughn · 2026-04-16T14:36:51Z

setting a default sorter that for data_objects.get ( or anytime running the iRODSDataObject constructor) sorts replicas of the data object first by the replica-"goodness" and secondly by reverse chronology of the replica modify_time (ie most recent first.) The replica at array position [0] will then determine the values of the fields discussed above.

Keep in mind that for a minor release, we cannot change the behavior of any public APIs. If the default sorter results in the output being different, then that's a no go. The default sorter must mirror the original behavior.

deciding for the time being not to deprecate anything. yet.

What are you referring to in regard to deprecation?

To me it makes natural sense to allow modify_time and replica_status to be accessed from the "head" object.

What does this mean?

d-w-moore · 2026-04-16T14:41:39Z

setting a default sorter that for data_objects.get ( or anytime running the iRODSDataObject constructor) sorts replicas of the data object first by the replica-"goodness" and secondly by reverse chronology of the replica modify_time (ie most recent first.) The replica at array position [0] will then determine the values of the fields discussed above.

Keep in mind that for a minor release, we cannot change the behavior of any public APIs. If the default sorter results in the output being different, then that's a no go. The default sorter must mirror the original behavior.

deciding for the time being not to deprecate anything. yet.

What are you referring to in regard to deprecation?

We'd discussed in the old issue/PR convo's whether we might not just deprecate the iRODSDataObject fields like replica status and modify_time that are really just a reflection of the corresponding attribute of replicas[0]

To me it makes natural sense to allow modify_time and replica_status to be accessed from the "head" object.

What does this mean?

Just that .replicas[0].FIELD is mirrored in .FIELD, but that is pretty natural.
I guess we could actually just make them properties, rather than duplicating the data. But that is low priority.

d-w-moore · 2026-04-16T14:45:42Z

@korydraughn - I'm fine with changing the default order back to sorting on replica number for this minor release, even if it will allow attributes such as dataObject.modify_time to continue to misrepresent the "information advertised" .... It's but a minor code change to allow the application writer to sort differently if they so desire....

korydraughn · 2026-04-16T15:55:18Z

We'd discussed in the old issue/PR convo's whether we might not just deprecate the iRODSDataObject fields like replica status and modify_time that are really just a reflection of the corresponding attribute of replicas[0]

Oh right. That still sounds like an acceptable approach.

Just that .replicas[0].FIELD is mirrored in .FIELD, but that is pretty natural. I guess we could actually just make them properties, rather than duplicating the data. But that is low priority.

I'm not yet convinced that is the proper approach. Feels like it should be handled via support functions which simplify the find-replica step.

Do instances of iRODSDataObject always have the list of replicas? If so, then they can sort/search the list of replicas for what they need. Perhaps that's how the iRODSDataObject constructor works in this PR?

d-w-moore · 2026-05-05T14:06:29Z

Have I addressed this already, @korydraughn ?

I'm not yet convinced that is the proper approach. Feels like it should be handled via support functions which simplify the find-replica step.

If not, perhaps you could elaborate? So should we be changing the interface in regards to how replicas are actually found; or do we actually only want one replica represented by any iRODSDataObject instance? I could not tell which interpretation you meant.

korydraughn · 2026-05-05T19:39:09Z

I don't know if you've addressed it.

If not, perhaps you could elaborate?

I'm highlighting that providing the list of replicas is all that's needed. We do not need to mirror properties from replica N to the iRODSDataObject instance. Doing that will be wasteful if the user doesn't use it. The user can filter the list of replicas however they like.

I didn't see a response to this question.

Do instances of iRODSDataObject always have the list of replicas? If so, then they can sort/search the list of replicas for what they need. Perhaps that's how the iRODSDataObject constructor works in this PR?

d-w-moore · 2026-05-06T15:47:32Z

I didn't see a response to this question.

Do instances of iRODSDataObject always have the list of replicas? If so, then they can sort/search the list of replicas for what they need. Perhaps that's how the iRODSDataObject constructor works in this PR?

The replica list is always there, and - yes - initialized and sorted in the iRODSDataObject constructor, which now takes the option of replica_sort_function. The sorting default doesn't change with this PR; it is still numerically by ascending replica number.

In this PR we at least allow the possibility of feeding in an alternative value for replica_sort_function , either directly into that constructor or indirectly via <session>.data_objects.get( path, ...). We also add _REPLICA_FITNESS_SORT_KEY_FN as a sort option (see the new test) which is different than the default sort order, in that it sorts primarily by the "goodness" of replica status and secondarily descending by mtime. This seems likely to become the default in v4.0.0.

korydraughn · 2026-05-06T15:59:29Z

Makes sense to me.

The leading underscore on the sort functions - consider making those public (i.e. remove leading underscore?). That will keep users from needing to implement their own version of them.

d-w-moore · 2026-05-06T16:29:26Z

Makes sense to me.

The leading underscore on the sort functions - consider making those public (i.e. remove leading underscore?). That will keep users from needing to implement their own version of them.

sounds good. and, will wait for tests to pass before squashing.

alanking · 2026-05-06T20:24:11Z

Please notify when ready for one last pass over.

d-w-moore · 2026-05-07T01:48:29Z

Please notify when ready for one last pass over.

addressing the ruff gopher-holes first. Then will squash and notify.

d-w-moore · 2026-05-08T11:49:54Z

Ok ... the ruff drumbeat is now silent. Except for now there's a couple of codacy warnings, which I'm choosing not to heed right now as they are very minor.

d-w-moore · 2026-05-08T12:11:39Z

squashed

korydraughn

Looks good. Just one question regarding a code comment.

korydraughn · 2026-05-08T16:13:38Z

Squash it.

The iRODSDataObject has thus far sorted its replicas list by ascending replica order. replica_sort_function is a new keyword option recognized by both <session>.data_objects.get() and the iRODSDataObject constructor, and either can be given an alternate value (REPLICA_FITNESS_SORT_KEY_FN) or even a user-defined sort key if desired in order to alter replica order to a way that conforms more to user requirements.

d-w-moore · 2026-05-14T12:54:11Z

request final review of doc string changes....

trel

two tweaks.

trel · 2026-05-14T13:02:11Z

+                control threads should be used.
+            updatables: a tuple or list in which each element is a tqdm-like progress bar object, or the equivalent:
+                an instance on which func.update(n) can be called with n being the number of bytes successfully
+                copied in the the data transfer increment just completed.  If not a tuple or list, this argument


Suggested change

copied in the the data transfer increment just completed. If not a tuple or list, this argument

copied in the data transfer increment just completed. If not a tuple or list, this argument

korydraughn · 2026-05-14T13:58:22Z

+            local_path: a filename within the local filesystem, the target for download ("GET") of the data
+                object if one is requested.  Directory components in the path must already exist.
+            num_threads: in the case of a parallel data transfer (for large files), specifies how many transfer
+                control threads should be used.
+            updatables: a tuple or list in which each element is a tqdm-like progress bar object, or the equivalent:
+                an instance on which func.update(n) can be called with n being the number of bytes successfully
+                copied in the the data transfer increment just completed.  If not a tuple or list, this argument
+                is interpreted as a single updatable.
+            replica_sort_function: a sort key function dictating the order of replica query results in 'self.replicas'


These parameters have default arguments which are not explained or mentioned. Please document what the behavior is when the defaults are used.

Co-authored-by: Terrell Russell <terrellrussell@gmail.com>

d-w-moore mentioned this pull request Apr 15, 2026

[#746] can now give (repl) row result sort function in DataObject constructor #747

Closed

d-w-moore force-pushed the 746.m branch from f00b6e7 to 4f9e624 Compare April 16, 2026 14:00

korydraughn reviewed Apr 16, 2026

View reviewed changes

Comment thread irods/test/data_obj_test.py Outdated

Comment thread irods/data_object.py

korydraughn reviewed Apr 16, 2026

View reviewed changes

Comment thread irods/test/data_obj_test.py Outdated

Comment thread irods/test/data_obj_test.py Outdated

d-w-moore force-pushed the 746.m branch 2 times, most recently from 0cc7227 to a9c4e99 Compare April 27, 2026 13:38

alanking reviewed Apr 27, 2026

View reviewed changes

Comment thread irods/test/data_obj_test.py

Comment thread irods/data_object.py Outdated

Comment thread irods/data_object.py Outdated

d-w-moore force-pushed the 746.m branch from a1d458f to 6b9f71d Compare May 6, 2026 16:41

d-w-moore force-pushed the 746.m branch 2 times, most recently from f7e6c10 to 07a9710 Compare May 8, 2026 12:10

korydraughn reviewed May 8, 2026

View reviewed changes

Comment thread irods/test/data_obj_test.py Outdated

korydraughn reviewed May 8, 2026

View reviewed changes

Comment thread irods/data_object.py

d-w-moore force-pushed the 746.m branch from 1991b2e to ac520e1 Compare May 12, 2026 12:39

d-w-moore added 3 commits May 13, 2026 22:23

README doc

a0fb111

[_SQ] document sort key fns, create new doc-string for data_objects.get

e9fc8b7

d-w-moore force-pushed the 746.m branch from bf918b7 to e9fc8b7 Compare May 14, 2026 02:25

ruff,docstring chg

49529c6

trel reviewed May 14, 2026

View reviewed changes

korydraughn reviewed May 14, 2026

View reviewed changes

d-w-moore and others added 2 commits May 14, 2026 23:56

Update README.md

a2d8715

Co-authored-by: Terrell Russell <terrellrussell@gmail.com>

explain defaults

dc9e0f7

	copied in the the data transfer increment just completed. If not a tuple or list, this argument
	copied in the data transfer increment just completed. If not a tuple or list, this argument

Conversation

d-w-moore commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

korydraughn commented Apr 16, 2026

Uh oh!

d-w-moore commented Apr 16, 2026

Uh oh!

d-w-moore commented Apr 16, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

korydraughn commented Apr 16, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

d-w-moore commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

korydraughn commented May 5, 2026

Uh oh!

d-w-moore commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

korydraughn commented May 6, 2026

Uh oh!

d-w-moore commented May 6, 2026

Uh oh!

alanking commented May 6, 2026

Uh oh!

d-w-moore commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

d-w-moore commented May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

d-w-moore commented May 8, 2026

Uh oh!

korydraughn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

korydraughn commented May 8, 2026

Uh oh!

Uh oh!

d-w-moore commented May 14, 2026

Uh oh!

trel left a comment

Choose a reason for hiding this comment

Uh oh!

trel May 14, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

korydraughn May 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

d-w-moore commented Apr 15, 2026 •

edited

Loading

d-w-moore commented May 5, 2026 •

edited

Loading

d-w-moore commented May 6, 2026 •

edited

Loading

d-w-moore commented May 7, 2026 •

edited

Loading

d-w-moore commented May 8, 2026 •

edited

Loading